Skip to content

fix: apply query variable defaults when undefined is passed explicitly - #13362

Closed
atharv-sys32 wants to merge 1 commit into
apollographql:mainfrom
atharv-sys32:fix/undefined-variable-defaults
Closed

fix: apply query variable defaults when undefined is passed explicitly#13362
atharv-sys32 wants to merge 1 commit into
apollographql:mainfrom
atharv-sys32:fix/undefined-variable-defaults

Conversation

@atharv-sys32

@atharv-sys32 atharv-sys32 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #13345

Summary by CodeRabbit

  • Bug Fixes
    • Corrected query handling when variables are omitted or explicitly set to undefined.
    • GraphQL default variable values are now applied consistently, improving conditional field selection and related query results.

@apollo-librarian

apollo-librarian Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

✅ AI Style Review — No Changes Detected

No MDX files were changed in this pull request.

Review Log: View detailed log

This review is AI-generated. Please use common sense when accepting these suggestions, as they may not always be accurate or appropriate for your specific context.

@changeset-bot

changeset-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 4411bea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: bba4d387-56ef-46cd-b80e-d83b6f96cf14

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The cache store reader now removes undefined variable entries before applying GraphQL query defaults. A test verifies that an explicitly undefined variable uses its declared default during conditional field evaluation.

Changes

Cache variable default handling

Layer / File(s) Summary
Normalize variables and validate defaults
src/cache/inmemory/readFromStore.ts, src/cache/inmemory/__tests__/readFromStore.ts
diffQueryAgainstStore filters undefined variables before merging query defaults, and a Jest test verifies default-driven @include behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: benjamn

Poem

I’m a bunny with variables light,
Undefined ones vanish from sight.
Defaults hop in place,
Fields show with grace,
And cache reads now get it just right.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main fix: applying query variable defaults when an undefined value is passed.
Linked Issues check ✅ Passed The code and test directly address #13345 by ignoring undefined variables so query defaults apply during cache resolution.
Out of Scope Changes check ✅ Passed The changes stay focused on undefined variable handling and a regression test, with no unrelated edits visible.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

When query variables like { show: undefined } are passed, the cache's
diffQueryAgainstStore would overwrite GraphQL-specified default values
with undefined by spreading the user-provided variables directly.
This caused @include/@Skip directives to fail with 'Invalid variable
referenced' errors.

Filter out undefined values before merging with defaults, matching the
behavior already implemented in QueryManager.getVariables.

Fixes #13345
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default value does not apply to variable passed as undefined

1 participant